Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ui: add connected components for insights #88419

Merged
merged 1 commit into from
Sep 27, 2022

Conversation

ericharmeling
Copy link
Contributor

@ericharmeling ericharmeling commented Sep 21, 2022

This commit adds connected components for the workload and schema insights pages, for use in the CC Console.

Fixes #87693.

https://www.loom.com/share/08163a7c125948119ca71ac097099a29

Release note: None

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@ericharmeling ericharmeling force-pushed the insights-pages-connected branch 5 times, most recently from aa7f92c to b05415f Compare September 26, 2022 19:10
@ericharmeling ericharmeling marked this pull request as ready for review September 26, 2022 21:54
@ericharmeling ericharmeling requested a review from a team September 26, 2022 21:54
@ericharmeling
Copy link
Contributor Author

ericharmeling commented Sep 26, 2022

FYI, the statement fingerprint details pages are returning an undefined error:

TypeError
Cannot read properties of undefined (reading 'apply')

Loom: https://www.loom.com/share/09deb18b98044502a7f41320081f7c8e

This commit adds connected components for the workload and schema insights pages, for use
in the CC Console.

Fixes cockroachdb#87693.

Release note: None
Copy link
Contributor

@maryliag maryliag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when do you see this error? is just on the browser when you open the details page or it doesn't even load the page?

Reviewed 35 of 37 files at r1.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @ericharmeling and @maryliag)


pkg/ui/workspaces/cluster-ui/src/store/sagas.ts line 40 at r1 (raw file):

    fork(transactionInsightsSaga),
    fork(transactionInsightDetailsSaga),
    fork(statementInsightsSaga),

to confirm: there is no statementInsightsDetailsSaga because we don't do a call for the details here like we do on the transactions, right?

Copy link
Contributor Author

@ericharmeling ericharmeling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @maryliag)


pkg/ui/workspaces/cluster-ui/src/store/sagas.ts line 40 at r1 (raw file):

Previously, maryliag (Marylia Gutierrez) wrote…

to confirm: there is no statementInsightsDetailsSaga because we don't do a call for the details here like we do on the transactions, right?

Yes! Exactly.

Copy link
Contributor

@maryliag maryliag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the error on the SQL Activity aside, the changes made here to make it available on CC Console Dedicated: :lgtm:

Reviewed 2 of 37 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @ericharmeling)

Copy link
Contributor Author

@ericharmeling ericharmeling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TFTR!

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @ericharmeling)

@ericharmeling
Copy link
Contributor Author

bors r+

@craig
Copy link
Contributor

craig bot commented Sep 27, 2022

Build succeeded:

@craig craig bot merged commit 03b8afb into cockroachdb:master Sep 27, 2022
@ericharmeling
Copy link
Contributor Author

FYI, the statement fingerprint details pages are returning an undefined error:

TypeError
Cannot read properties of undefined (reading 'apply')

Loom: https://www.loom.com/share/09deb18b98044502a7f41320081f7c8e

Following up on this error:

It does not affect DB Console, and it does not occur in the commits before this PR.

To reproduce:

(from cockroach):

git checkout 15e83552f6d498a308897b85dbdf1424a4b52e9a
./dev build
./dev ui watch

(from managed-service):

git checkout master
./scripts/crl-vault-shell staging
make console-symlink-cluster-ui CLUSTER_UI_VERSION=22-2 GOPATH=$HOME/go CLUSTER_UI_PATH=~/go/src/github.com/cockroachdb/cockroach/_bazel/bin/pkg/ui/workspaces/cluster-ui
yarn --cwd console start:staging --env watch_cluster_ui

Navigate to localhost:8081 -> sql-obs-latest -> SQL Activity -> select any statement fingerprint

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Insights page added to Cloud Dedicated
3 participants